Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/claire elastic test #153

Merged
merged 100 commits into from
Oct 14, 2024
Merged

Feat/claire elastic test #153

merged 100 commits into from
Oct 14, 2024

Conversation

claire-lemon
Copy link

test: test searchEngine Instances

@claire-lemon claire-lemon self-assigned this Jul 18, 2024
Copy link
Member

@steve-lemon steve-lemon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 정리 좋아요. 좀더 개선해 봅시다.

src/cores/elastic/elastic6-service.ts Outdated Show resolved Hide resolved
src/cores/elastic/elastic6-service.ts Outdated Show resolved Hide resolved
src/cores/elastic/elastic6-service.ts Outdated Show resolved Hide resolved
Copy link
Member

@steve-lemon steve-lemon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 정리하고 가쥬

src/cores/elastic/elastic6-service.ts Outdated Show resolved Hide resolved
src/cores/elastic/elastic6-service.ts Outdated Show resolved Hide resolved
src/cores/elastic/elastic6-service.ts Outdated Show resolved Hide resolved
src/cores/elastic/elastic6-service.ts Show resolved Hide resolved
src/cores/elastic/elastic6-service.spec.ts Outdated Show resolved Hide resolved
src/cores/elastic/elastic6-service.spec.ts Outdated Show resolved Hide resolved
@steve-lemon
Copy link
Member

추가적으로 검토할일
@elastic/elasticsearch 요거 업데이트하기.

@claire-lemon claire-lemon requested a review from steve-lemon July 23, 2024 09:31
@claire-lemon
Copy link
Author

claire-lemon commented Jul 24, 2024

어제 리뷰해주셨던 부분들 수정했습니다.

  1. '2.13' 버전 특정지었던 부분들을 'version < 7' 범위 적용
  2. params: any 부분에서, any 안쓰도록 interface 새로 추가 후 적용
  3. 테스트 메소드 이름 수정

추가적으로,
searchRaw의 response에서 os2.13만 '_type': '_doc'가 존재하지 않아 테스트를 통과하지 못하고 있습니다.

ElasticSearch Document에 따르면, 'typeless APIs will return the dummy type _doc in the response'
7.X 버전에서는 더미 데이터로 '_type': '_doc'를 return한다고 적혀있지만
최신 openSearch 버전에서는 더미데이터 '_type': '_doc'를 제외한 채로 return하는 것 같습니다.

버전을 명확하게 나누기 위해 좀 더 찾아보겠습니다!

Copy link
Member

@steve-lemon steve-lemon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고했어요~

src/cores/elastic/elastic6-service.spec.ts Outdated Show resolved Hide resolved
src/cores/elastic/elastic6-service.ts Outdated Show resolved Hide resolved
src/cores/elastic/elastic6-service.ts Outdated Show resolved Hide resolved
src/cores/elastic/elastic6-service.ts Outdated Show resolved Hide resolved
src/cores/elastic/elastic6-service.ts Outdated Show resolved Hide resolved
src/cores/elastic/elastic6-service.ts Outdated Show resolved Hide resolved
@claire-lemon
Copy link
Author

claire-lemon commented Jul 24, 2024

  1. 에러 관련된 부분 수정
    : 로깅 더 상세하게 할 수 있도록, 에러 발생시 패턴대로 처리하도록
  2. comment 작성
  3. openSearch 버전이 2.0 이상이면 '_type': '_doc' 빼도록
    참고한 자료: Remove mapping type usage opensearch-project/OpenSearch#2979

리뷰해주신 부분들 수정했습니다!

@claire-lemon
Copy link
Author

  1. updateItem 수정
    1. index()를 활용한 update overwrite

      script overwrite 기능에 의한 업데이트 성능 저하가 우려돼, index()를 활용하는 방식으로 코드를 수정했습니다.
      각각의 코드(script를 활용한 업데이트, index를 활용한 업데이트)로 basicCRUDTest update 부분의 테스트 소요 시간을 측정해보았는데, 적게는 0.05ms, 많게는 0.4ms까지 차이가 나는 것을 확인했습니다. 따라서 더 퍼포먼스가 좋은 index를 활용하는 방식으로 수정했습니다

    2. 낙관적 동시성 제어

      1. 6.0 이후부터는 _version이 아닌 _seq_no, _primary_term으로 낙관적 동시성을 제어함을 확인했습니다. 따라서 _seq_no로 개별 작업 단위의 동시성을 제어하고, _primary_term로 샤드 수준의 동시성을 제어할 수 있도록 했습니다.
  2. doTest로 테스트 함수 호출 부분 모듈화
    1. 버전 별로 테스트 함수들을 호출하는 부분이 길고 중복된다고 판단해 doTest()로 대체했습니다.
  3. comment 추가
    1. comment가 없었거나 부족하다고 느껴지는 부분들에 comment를 추가했습니다.

@claire-lemon
Copy link
Author

  1. updateItem readback check 추가
  2. updateItem시 overwrite되지 않고 saveItem시에만 overwrite 되도록 수정
  3. updateItem에서 array increment 지원하도록 개선 + 관련 테스트 추가
  4. buffer를 이용한 dummy updateItem에서도 increment 지원하도록 개선
  5. ElasticIndexService에서 ES6 version checking 하는 부분들을 제거
    → ES6 version checking이 필요한 부분은 Elastic6Service에 overriding하여 구현
  6. 테스트 코드 개선 및 정리
    테스트 과정과 결과들을 깔끔하게 확인할 수 있도록 수정
    - agent 활용
    - 객체 결과의 경우 key 별로 검증

@claire-lemon
Copy link
Author

  1. _ver()로 버전 잘 업데이트되는지 확인하도록
  2. extra의 경우 overwrite되도록
  3. increment array testcase 추가
  4. ElasticIndexService 추상화
  5. test에서 any 사용하지 않도록 CRUDModel interface 추가
    GeneralItem은 null이나 undefined를 지원하지않아서 optional 처리가 되지 않습니다.
    $id?: string | any; 따라서 이런 식으로 해뒀는데, 보기 안좋아서 다른 방법이 있으면 수정해보겠습니다!

Copy link
Member

@steve-lemon steve-lemon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@steve-lemon steve-lemon merged commit f068db9 into develop Oct 14, 2024
@steve-lemon steve-lemon deleted the feat/claire-elastic-test branch October 14, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants